I am a bit at a loss when it comes to the ffi checks. Pointers welcome

AFAIK the header that contains this in CPython is Include/internal/pycore_dict.h, so you'd need to include that path.

(Not opining on any part of this patch.)

Thanks I will try that.

So the ffi-check situtation is a bit more complex since accessing the core headers requires Py_BUILD_CORE define. Is this something pyo3 maintainers would be willing to accept ?

So the ffi-check situtation is a bit more complex since accessing the core headers requires Py_BUILD_CORE define. Is this something pyo3 maintainers would be willing to accept ?

We should not be using any symbols from Include/internal.

I think this should now look better. I squashed the previous change to reduce the noise. so running just the lib test was too light. I will continue tomorrow.

Thanks for the review. I have been fighthing a bit cfg when it comes to the limited API. I will have to look into your suggestions.

I did a first pass in addressing the review and got clean clippy and test execution locally. The wrapper is now available without the limited API and the cfg are a bit more tiddy. I will check the CI run tomorrow and see if I missed anything else.

Merging this PR will not alter performance

✅ 140 untouched benchmarks

_{Comparing MatthieuDartiailh:frozendict (363cd08) with main (c85bc26)}

Thanks for the early reviews @Icxolu

I think I now have everything in order (including the cfgs).

As a possible followup we can add support for a PyAnyDict, where both PyDict & PyFrozenDict have a Deref impl to PyAnyDict. This will remove some duplications from both types.

I rebased and addressed all review comments. Regarding the constructor, given the limited functionality provided by CPython I went with the safe solution of creating an intermediate Python List. A builder may be feasible following PyFrozenSet but it means mutating frozen object which I am even surprised to see working in the set case.

I will try to work in a follow up PR on the PyAnyDict idea this week-end.

ruff is unhappy about some files I did not touch...

Rebasd on latest main

@bschoenmaeckers Could you elaborate a little on ?

As a possible followup we can add support for a PyAnyDict, where both PyDict & PyFrozenDict have a Deref impl to PyAnyDict. This will remove some duplications from both types.

Trying to implement Deref leads into conflict with the PyAny deref and does not match any existing pattern in pyo3 (there is not an AnySet for example). I am willing to work on this but I really not sure how to attack this in way that actually reduces code duplication.

I am willing to work on this but I really not sure how to attack this in way that actually reduces code duplication

Maybe introduce a PyAnyDictMethods trait and make PyDictMethods and PyFrozenDictMethod extend it

I can do that but what I am missing is how to avoid duplicating the implementation (since the PyAnyDict_Methods impl block will be identical for PyAnyDict, PyFrozenDict and PyDict). I could use a macro but I am wondering if @bschoenmaeckers had something else in mind.

To go into the Deref direction maybe something like:

#[repr(transparent)]
pub struct PyAnyDict(PyAny);

pyobject_native_type_core!(
    PyAnyDict_Check,
   ...
); // Not sure the macro will work, you might have to implement some traits yourself or with more specialized macros (if it's the case, just read pyobject_native_type_core implementation)

impl PyAnyDictMethods for Bound<'_, PyAnyDict> {
   fn ex(&self) {}
}

impl Deref for Bound<'_, PyFrozenDict> {
   type Target = Bound<'_, PyAnyDict>;
   fn deref(&self) -> &Bound<'_, PyAnyDict> { unsafe { self.cast_unchecked() } }
}

But I am not sure if it's actually doable.

The trait option would be to have something like the following instead of the Deref:

impl PyAnyDictMethods for Bound<'_, PyFrozenDict> {
   fn ex(&self) { unsafe { self.cast_unchecked::<PyAnyDict>() }.ex() }
}

@bschoenmaeckers Could you elaborate a little on ?

As a possible followup we can add support for a PyAnyDict, where both PyDict & PyFrozenDict have a Deref impl to PyAnyDict. This will remove some duplications from both types.

Trying to implement Deref leads into conflict with the PyAny deref and does not match any existing pattern in pyo3 (there is not an AnySet for example). I am willing to work on this but I really not sure how to attack this in way that actually reduces code duplication.

I experimented in this commit: 2aa4eb3.

I had to remove the DerefToPyAny for PyDict to avoid the conflict. But this is fine as it wil dreef trough the anydict to pyany.

If you want you can pick this up and create a mr

Thanks @bschoenmaeckers . Since the commit has what looks like many unrelated changes, I will re-apply the idea in a fresh commit. Should that remaion a follow-up or should I include as part of this PR ? And would a similar PR for AnySet make sense ?